-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
React: Upgrade to the new JSX transform #61692
Conversation
f6c8ec4
to
1b8dc96
Compare
Size Change: -1.13 kB (-0.06%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
It's interesting to see that this PR actually works as is but it's probably bundling "react/jsx-runtime" in all of our JS scripts that use jsx. |
1307b7f
to
705559a
Compare
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/client-assets.php |
705559a
to
2713e68
Compare
tools/webpack/development.js
Outdated
@@ -46,4 +46,33 @@ module.exports = [ | |||
} ), | |||
], | |||
}, | |||
{ | |||
...sharedConfig, | |||
name: 'react-jsx-runtime', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to need someone to check that the built script for this is correct. I'm trying to extract it to its own script and using "React" as an external global variable. I think it's ok now but a confirmation would be great.
@@ -75,21 +75,10 @@ module.exports = ( api ) => { | |||
], | |||
plugins: [ | |||
require.resolve( '@wordpress/warning/babel-plugin' ), | |||
[ | |||
require.resolve( '@wordpress/babel-plugin-import-jsx-pragma' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to deprecate this and ensure a breaking change release for babel-preset-default and scripts packages.
Flaky tests detected in eed6746. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9108633575
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
5c8d1ec
to
d7d8cd5
Compare
f5932fe
to
7cffbfe
Compare
We can still align as closely as possible with core in the production build, I don't see much value in dev mode though. As you pointed out in the testing instructions, e2e tests should basically cover everything. Now that I recall from the original PR, I think this ties to the way we currently do for our dev builds too. For instance, we don't really need to run a two-step build (babel -> webpack) for development. If we use only webpack in dev mode then it'll be a lot faster and enable advanced features like Fast Refresh. The production build can still be the same. This workflow could also make the transition to vite or other modern build tools easier too. It's really a bigger discussion that we had multiple times in different places though 😅. I understand that this is not the focus of this PR, but just want to state the possibility that we can still support |
Is it possible to ship a different source for
|
@Mamaduka That won't work because for both modes (dev and prod), the packages ship with |
So I think this PR is ready, should we move forward with it? What's left? |
@youknowriad, are aiming to ship this with WP 6.6? |
@Mamaduka That's my hope yes, that way third-party developers could start fixing the warnings... sooner. |
See WordPress/gutenberg#61692. See #6678. Props youknowriad. Fixes #61324. git-svn-id: https://develop.svn.wordpress.org/trunk@58271 602fd350-edb4-49c9-b593-d223f7449a82
I am concerned that this PR might have caused issues when using the scripts package in custom block development. Please see this comment. |
Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: gziolo <gziolo@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: sirreal <jonsurrell@git.wordpress.org> Co-authored-by: jsnajdr <jsnajdr@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org>
@youknowriad Heads up, as @t-hamano warns, we're seeing breakage too with |
Please refer to the following issue for the issue you are concerned about. The following dev notes may also be helpful. https://make.wordpress.org/core/2024/06/06/jsx-in-wordpress-6-6/ |
Alternative to #34221
What
React 17 introduced a new JSX runtime but we are still using the old JSX build process. As part of the React 19 upgrade, the React team is deprecating the old JSX so this is the second attempt to move away from it.
The initial PR #34221 tried to include the runtime into the "@wordpress/element" package but since, we've moved away from using @wordpress/element as the JSX target of our builds, which means we can take a simpler approach and just "externalize" the react/jsx-runtime script into a new WP script handle and global variable and use it while building our scripts.
We should be able to deprecate our package
@wordpress/babel-plugin-import-jsx-pragma
.To learn more about the new JSX transform, read this post.
This definitely requires a dev note to nudge plugin authors to also switch to the upgraded "@wordpress/scripts".
Todo:
Anything I'm missing?
Testing instructions.
The e2e tests got us covered here in general.